-
Notifications
You must be signed in to change notification settings - Fork 567
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
updates capa_explorer.py, enabling the user to choose b/w having bookmarks & comments. #2029
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased)
section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed
CHANGELOG updated or no update needed, thanks! 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased)
section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed
@Atlas-64 thank you for your contribution! Let's get the lint errors fixed before we review. Have you followed the capa development installation steps? Specifically, these steps outline how to use |
hi @mike-hunhoff i did follow the steps to install my version of capa (in editable mode) and the dependencies and then continued to run the pre commit command to see if it passes the linting issues but i cant seem to pinpoint what do we do to import askChoices for use in capa_ghidra() which is why it seems that on the ruff hook it throws me the following error : |
Thank you for your thorough explanation! You're encountering a side effect of the way Ghidrathon adds Ghidra FlatProgramAPI methods (e.g. |
CHANGELOG updated or no update needed, thanks! 😄
4afe1cc
to
1ffa18c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @Atlas-64 ! I've left comments for you to review. Please post any questions here and re-request my review when you're ready.
capa/ghidra/capa_explorer.py
Outdated
if user_choice == "bookmarks": | ||
for item in parse_json(capa_data): | ||
item.bookmark_functions() | ||
elif user_choice == "comments": | ||
for item in parse_json(capa_data): | ||
item.label_matches() | ||
elif user_choice == "both": | ||
for item in parse_json(capa_data): | ||
item.bookmark_functions() | ||
item.label_matches() | ||
else: | ||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This script modifies a Ghidra database by adding:
- new namespace named "capa" and corresponding namespace entries
- pre/plate-comments
- function bookmarks
We want our changes here to enable users to select any number of these options when running this script. Presently, the label_matches
method creates the "capa" namespace, corresponding namespace entries, and pre/plate-comments. We need to modify the label_matches
method to account for the user's selection and I'd recommend passing new boolean arguments to the label_matches
method to implement this. Creating a new "capa" namespace and corresponding namespace entries should be grouped as one option, likewise with setting pre/plate-comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So just to clarify for the adding comments based on the user input, we want to just have a boolean default and pass an argument based on what the user wants in the function call within the if-else statement.
and, if I understand right you'd like separate options for creating the 'capa' namespace and setting pre/plate comments, even though the namespace might be used for comments. Is that correct?
Can you elaborate on the reasoning behind separate options? Is it for user flexibility or maybe workflow reasons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taking a step back, it may be easier to understand the requested changes by reviewing the README's UI integration section.
capa_explorer.py does three things:
- Add Ghidra top-level namespace named "capa" that is viewable in Ghidra's Symbol Tree window
- Add pre/plate comments that are viewable in Ghidra's Disassembly Listing and Decompiler windows
- Add bookmarks that are viewable in Ghidra's Bookmarks window
We'd like to give user's an option to choose which of these three things are executed based on their needs/workflow. For example, a user may be interested in options 1 and 3 while not wanting 100s of comments added to their Ghidra database by option 2.
…64/capa into comments-bookmarks-option
capa/ghidra/capa_explorer.py
Outdated
for item in parse_json(capa_data): | ||
item.bookmark_functions() | ||
item.label_matches() | ||
item.bookmark_functions(bookmarks) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's simply not call bookmark_functions
here if bookmarks
is False
.
capa/ghidra/capa_explorer.py
Outdated
@@ -137,84 +137,114 @@ def set_pre_comment(self, ghidra_addr, sub_type, description): | |||
else: | |||
return | |||
|
|||
def label_matches(self): | |||
def label_matches(self, namespace=False, comments=False): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's split this into two functions, create_capa_namespace
and create_capa_comments
, that are only called if the corresponding bool values retrieved from the user are True
.
capa/ghidra/capa_explorer.py
Outdated
item.bookmark_functions() | ||
item.label_matches() | ||
item.bookmark_functions(bookmarks) | ||
item.label_matches(namespace, comments) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See my comment above about splitting this function in two.
@mike-hunhoff sorry for the absence have been busy with school, I think I will try running the changed version and see if its working alright, and then move on to actually running the tests against it . Will post screenshots here of the results |
Updates capa_explorer.py to let users choose between adding bookmarks/comments when the script is executed. Uses the Ghidra's askChoices API to do so.
closes issue: #1977
Checklist